Fix wordlist corruption from secure wipe of shared BIP39 string objects#335
Fix wordlist corruption from secure wipe of shared BIP39 string objects#3353rdIteration merged 3 commits intodevfrom
Conversation
- Normalize mnemonic words to lowercase in Seed.__init__ so BIP39 seeds are created correctly regardless of input case - Make detect_segment_type case-insensitive and whitespace-tolerant when matching BIP39, 4-letter, and SLIP-39 mnemonics - Normalize case and whitespace in SeedQrDecoder.add for SEED__MNEMONIC and SEED__FOUR_LETTER_MNEMONIC QR types - Add tests for case-insensitive seed creation and QR detection Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
wipe_string() uses ctypes.memset to zero string memory for security, but pending mnemonic words were stored as references to the shared global bip39.WORDLIST strings. When discard_pending_mnemonic() called wipe_list(), it corrupted the wordlist, causing "abandon" (and other entered words) to become null bytes and disappear from the keyboard word selection on subsequent seed entries. Fix: use "".join(word) to create independent string copies in update_pending_mnemonic() and update_pending_slip39_share() so wipe_list() only zeros the copies, not the shared wordlist entries. Co-authored-by: 3rdIteration <2230318+3rdIteration@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes regressions in seed entry/QR scanning caused by secure wiping of shared wordlist strings and improves input normalization so BIP39 mnemonics are handled case-insensitively and with flexible whitespace.
Changes:
- Prevent global wordlist corruption by copying mnemonic/share words before storing them for later secure wipe.
- Normalize BIP39 mnemonic input to lowercase during
Seedconstruction (case-insensitive entry). - Make text-QR mnemonic detection/parsing case-insensitive and whitespace-tolerant, with regression tests added.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/seedsigner/models/seed_storage.py |
Copies stored mnemonic/SLIP39 words to avoid corrupting shared/global wordlist strings during wipe. |
src/seedsigner/models/seed.py |
Lowercases normalized mnemonic input so mixed/upper-case words are accepted. |
src/seedsigner/models/decode_qr.py |
Lowercase + whitespace-tolerant splitting for mnemonic detection and Seed QR parsing. |
tests/test_seed.py |
Adds tests for case-insensitive mnemonics and regression test for wordlist corruption. |
tests/test_decodepsbtqr.py |
Adds tests for case-insensitive and whitespace-tolerant mnemonic text QR decoding. |
| raise Exception(f"index {index} is too high") | ||
| self._pending_mnemonic[index] = word | ||
| # Create an independent copy so that wipe_list() in | ||
| # discard_pending_mnemonic() won't corrupt the shared | ||
| # global wordlist strings via wipe_string/ctypes.memset. | ||
| self._pending_mnemonic[index] = "".join(word) |
There was a problem hiding this comment.
update_pending_mnemonic() is called with word=None in ToolsCalcFinalWord* flows (e.g., to clear the last slot before re-entry). After this change, "".join(word) will raise TypeError on None, breaking that path. Handle None explicitly (store None without copying) before calling join().
| if all(x in wordlist for x in s.strip().lower().split()): | ||
| # checks if all words in list are in bip39 word list | ||
| return QRType.SEED__MNEMONIC | ||
|
|
||
| elif all(x in _4LETTER_WORDLIST for x in s.strip().split(" ")): | ||
| elif all(x in _4LETTER_WORDLIST for x in s.strip().lower().split()): | ||
| # checks if all 4 letter words are in list are in 4 letter bip39 word list | ||
| return QRType.SEED__FOUR_LETTER_MNEMONIC | ||
|
|
||
| elif all(x in slip39_wordlist for x in s.strip().lower().split(" ")): | ||
| elif all(x in slip39_wordlist for x in s.strip().lower().split()): | ||
| return QRType.SEED__SLIP39 |
There was a problem hiding this comment.
detect_segment_type() repeats s.strip().lower().split() three times. Consider normalizing once (e.g., precompute the split words list) to reduce duplication and keep the seed-type checks easier to maintain.
Description
Three related bugs when entering/scanning BIP39 recovery phrases:
InvalidSeedException ValueErrorwhen entering mixed-case mnemonics (e.g. "Abandon").Root cause (bug 1)
wipe_string()usesctypes.memsetto zero Python string internals.SeedStorage._pending_mnemonicstored direct references tobip39.WORDLISTstring objects. Whendiscard_pending_mnemonic()→wipe_list()ran, it corrupted the global wordlist:Changes
seed_storage.py: Use"".join(word)inupdate_pending_mnemonic()andupdate_pending_slip39_share()to store independent string copies.str(word)andword[:]return the same object in CPython — only"".join()creates a genuinely new string that can be safely wiped.seed.py: Add.lower()to NFKD normalization inSeed.__init__so mnemonic words are case-insensitive.decode_qr.py: Use.lower().split()instead of.split(" ")indetect_segment_type()andSeedQrDecoder.add()for case-insensitive, whitespace-tolerant QR detection.No UI changes — all fixes are in input normalization and storage logic.
This pull request is categorized as a:
Checklist
pytestand made sure all unit tests pass before sumbitting the PRIf you modified or added functionality/workflow, did you add new unit tests?
I have tested this PR on the following platforms/os:
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.